Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Conversation

Copy link

netlify bot commented Mar 14, 2025

Deploy Preview for docs-cluster-to-cluster-sync ready!

Name Link
🔨 Latest commit cb5f4e0
🔍 Latest deploy log https://app.netlify.com/sites/docs-cluster-to-cluster-sync/deploys/67e178e822dcde00099f2234
😎 Deploy Preview https://deploy-preview-670--docs-cluster-to-cluster-sync.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@gmiller-mdb gmiller-mdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly LTGM, with a few questions!

@mayaraman19 mayaraman19 requested a review from gmiller-mdb March 18, 2025 15:34
Copy link
Collaborator

@gmiller-mdb gmiller-mdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment

@mayaraman19 mayaraman19 requested a review from gmiller-mdb March 18, 2025 18:47
Copy link
Collaborator

@gmiller-mdb gmiller-mdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

@nickweinberger nickweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few suggestions–please let me know if you have any questions or if I can clarify anything!

Copy link

@nickweinberger nickweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few more comments for you, Maya. Mostly small suggestions to make the wording as clear as possible, and one larger structural suggestion to clarify the requirements for the destination cluster's balancer, so that piece doesn't get lost. Thanks for the back and forth on this!

Copy link
Collaborator

@gmiller-mdb gmiller-mdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

@nickweinberger nickweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM! I left a few non-blocking comments and suggestions just to make it as clean and clear as possible, but I'll leave those up to your judgement. Thanks for all your work on this Maya!

Comment on lines +10 to +11
and you are not running ``mongosync`` with :ref:`namespace
filtering <c2c-filtered-sync>`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and you are not running ``mongosync`` with :ref:`namespace
filtering <c2c-filtered-sync>`,
and you are running ``mongosync`` without :ref:`namespace
filtering <c2c-filtered-sync>`,

Comment on lines +21 to +22
See :ref:`disabling-balancer-filtered`. You can also fully disable
the source cluster's balancer.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! thanks for adding this.

Comment on lines +27 to +29
filter, do not run :dbcommand:`shardCollection` on collections
within the namespace filter. If you run :dbcommand:`shardCollection` on
collections within the namespace filter during the migration, ``mongosync``

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
filter, do not run :dbcommand:`shardCollection` on collections
within the namespace filter. If you run :dbcommand:`shardCollection` on
collections within the namespace filter during the migration, ``mongosync``
filter, running :dbcommand:`shardCollection` on collections within the namespace filter during the migration causes ``mongosync`` to return

If possible to make these two sentences into one I think it might be cleaner, but up to your judgement–if you like to be explicit "do not run X command" that's ok too.

If you are using a :ref:`namespace filter <c2c-filtered-sync>`
and want to enable your source cluster's balancer for
collections outside the namespace filter,
use the following instructions before

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use the following instructions before
follow these instructions before

starting ``mongosync``. This gives the cluster time to
finish any in progress chunk migrations.
- If the source or destination cluster is a sharded cluster
and you are not running ``mongosync`` with :ref:`namespace

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as other comment: I'd suggest "you are running mongosync without..."

- If you have enabled the source cluster's
balancer, but disabled it for collections within the namespace
filter, do not run :dbcommand:`shardCollection` on collections
within the namespace filter. If you run :dbcommand:`shardCollection` on

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other comment on Behavior page–if possible to combine into one sentence to reduce repetition that would be great, but up to your discretion.

@mayaraman19 mayaraman19 merged commit b4f6334 into mongodb:master Mar 24, 2025
4 checks passed
@mayaraman19 mayaraman19 deleted the DOCSP-46644-stop-balancer branch March 24, 2025 15:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants